feat: add native GroupsAccumulator for any_value - #23065
Conversation
Signed-off-by: Kevin-Li-2025 <2242139@qq.com>
…s-accumulator # Conflicts: # datafusion/functions-aggregate/Cargo.toml
github-actions
Bot
added
sqllogictest
|
|
||
| #[derive(Debug)] | ||
| struct AnyValueGroupsAccumulator { | ||
| values: Vec<ScalarValue>, |
There was a problem hiding this comment.
It could be worth specializing for primitive / binary (view) types to save further memory / allocations
|
Addressed the primitive specialization feedback by adding a native primitive any_value groups accumulator for numeric, decimal, temporal, and timestamp types. The new path stores group state as a native Vec<T::Native> plus the existing is_set bitmap, and only falls back to the ScalarValue-based accumulator for non-primitive/complex types. This keeps the existing generic behavior while avoiding per-group ScalarValue state for the common primitive cases. Validation run locally:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23065 +/- ##
==========================================
- Coverage 80.75% 80.73% -0.02%
==========================================
Files 1096 1096
Lines 373605 374030 +425
Branches 373605 374030 +425
==========================================
+ Hits 301713 301985 +272
- Misses 53890 53998 +108
- Partials 18002 18047 +45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
# Conflicts: # datafusion/functions-aggregate/Cargo.toml
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
3 participants
Which issue does this PR close?
Follow-up to #23043, which is now merged.
Rationale for this change
Grouped
any_valuefalls back toGroupsAccumulatorAdapter, creating one boxed accumulator per group and materializing per-group slices. That overhead dominates high-cardinalityGROUP BYworkloads even thoughany_valueonly retains one non-null value per group.What changes are included?
GroupsAccumulatorthat scans each batch once, supports filters and partial-state merging, preserves the existing two-column state contract, supportsEmitTo::First, and implementsconvert_to_state.Vec<T::Native>state for numeric, decimal, temporal, and timestamp Arrow primitive types.ScalarValueper group.ScalarValuefallback for remaining supported Arrow types.GroupsAccumulatorAdapter.This completes the review suggestion to specialize primitive and binary/view types.
Validation
cargo fmt --all --checkcargo test -p datafusion-functions-aggregate --lib(164 passed)cargo clippy -p datafusion-functions-aggregate --all-targets --no-deps -- -D warningscargo bench -p datafusion-functions-aggregate --bench any_value -- --noplotgit diff upstream/main --checkfor the five-file PR diffLocal Apple Silicon benchmark medians (8,192 rows / 4,096 groups):
Running Clippy without
--no-depsis currently blocked by an unrelated dead-code warning forDynamicFilterPhysicalExpr::from_partson currentmain; the target crate itself passes with warnings denied.AI assistance
AI tooling assisted with implementation and validation. I reviewed the final accumulator/state flow, the reviewer feedback, and the benchmark design, and ran the validations listed above.